fix: schedules, restore direct update semantics and strengthen parsin…#205
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
bradhe
left a comment
bradhe
left a comment
There was a problem hiding this comment.
Another amazing improvement, thanks @burakdede ! We owe you a tshirt at least 😉 . Will leave this open for a day or so before landing it.
| Arg::new("schedule_id") | ||
| .required(true) | ||
| .value_parser(value_parser!(String)) | ||
| .help("The schedule ID to delete") | ||
| .action(clap::ArgAction::Set), |
There was a problem hiding this comment.
Ah this is a good reminder that we need to support deleting schedules by name 😄
| Arg::new("schedule_id") | ||
| .required(true) | ||
| .value_parser(value_parser!(String)) | ||
| .help("The schedule ID to update") | ||
| .action(clap::ArgAction::Set), |
There was a problem hiding this comment.
This actually can be by id or by name now 😄 https://docs.tower.dev/docs/reference/api/update-schedule. With name, you have to have --environment.
We have a lot of confusion about parameters with our users. People get confused by the differences in how we accept name/ID parameters:
tower schedules create --name=my-schedule --environmnet=production --cron="*/5 * * * *"
and
tower schedules update my-schedule --environmnet=production --cron="*/5 * * * *"
Something for us to address...
socksy
left a comment
socksy
left a comment
There was a problem hiding this comment.
Ah good change! I'm working on exactly this clap code locally (removing allow_external_subcommands where possible since it seems to break so many default behaviours of clap) so let's get this in ASAP so I can rebase on it :)
3 participants
…g tests
This PR fixes the schedule update CLI behavior reported with #167
schedules updateargument parsing was semantically wrong due toallow_external_subcommands, so the schedule ID/flags could be parsed unreliably.Possible backend issues still persisting;
not-a-cronfor PUT/schedules/{id}.